-
-
Notifications
You must be signed in to change notification settings - Fork 13.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
installer: Don't run graphical installer ISOs as root #42610
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, did not know about GNOME installer, might be interesting to people at #41797
chmod a+rx /root/Desktop/gnome-terminal.desktop | ||
cp ${pkgs.gparted}/share/applications/gparted.desktop /root/Desktop/gparted.desktop | ||
chmod a+rx /root/Desktop/gparted.desktop | ||
ln -sfT ${pkgs.gnome3.gnome-terminal}/share/applications/gnome-terminal.desktop /home/nix-live/Desktop/gnome-terminal.desktop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GNOME does not have desktop by default so I guess this is moot. You would need something like:
services.xserver.desktopManager.gnome3 = {
extraGSettingsOverridePackages = with pkgs; [ gnome3.gnome_shell ];
extraGSettingsOverrides = ''
[org/gnome/shell]
favorite-apps=['firefox.desktop', 'nixos-manual.desktop', 'gparted.desktop', 'org.gnome.Terminal.desktop']
'';
};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. My solution for now is to remove gnome desktop entries and let someone more familiar with gnome take care of improving that installer.
Thanks for going off of my idea. ❤️ |
@@ -27,47 +18,15 @@ with lib; | |||
synaptics.enable = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be libinput
? I don't think it's a default for plasma5.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. I didn't want to make any major functionality changes right now.
Running as non-root is major enough for a single PR and warrants consideration purely on it's own rights.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No resistence here.
I don't think a gnome wayland session would have ever worked with root login if there ever was interest to have GNOME the default DE for the graphical installation media.
Other things are:
-
perhaps a default password
-
please remove
cd-dvd/installation-cd-graphical-kde-new-kernel.nix
here.
I don't think it's actually used for anything.
enable = true; | ||
|
||
# Don't start the X server by default. | ||
autorun = mkForce false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drop this I think. This is a graphical media, I would expect it to just start.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this belongs in another PR.
I think we should add another boot entry so you can still choose not to start the graphical environment, at the same time we make the one that autostarts the graphical environment the default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should add another boot entry so you can still choose not to start the graphical environment, at the same time we make the one that autostarts the graphical environment the default.
Why would that be needed? I don't see the graphical env ever being not functional, and you can always just start it and switch to a tty if needed. If someone wanted to not have a graphical env they would use the other iso...
Yeah a discussion for another PR. 👍
Great work! I think the user should be less complex, like just 'live' without |
3a088a7
to
ff7247c
Compare
@davidak Thanks for your feedback. I agree. |
This is currently causing issues with some tools like Dolphin (the KDE file manager) that refuses to run as root. We can instead allow passwordless sudo with similar effect.
ff7247c
to
b48a82f
Compare
@@ -63,11 +34,23 @@ with lib; | |||
Icon=text-html | |||
''; | |||
|
|||
# Replace default gparted desktop file with one that does "sudo gparted" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't this block be useful for both Gnome and KDE images?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gnome doesn't have files on the desktop anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A desktop file is not a file on the desktop, a desktop file is still used to launch applications from launchers etc.
And from my point of view it would possibly be useful if a user could click "gparted" in the launcher and that it's executed with sudo. But I'm not sure if there's any magic involved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But these entries would not be visible any more since gnome doesn't read ~/Desktop
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You suggest this would be useful if it used a patched version of gparted that uses sudo?
Currently here it only copies this file to the desktop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@worldofpeace Yeah, sure. That would be more useful. Especially on an installation media.
Not sure if that's in scope of this PR. But maybe it should be since the behaviour is changed from "I can click that button" to "Clicking that button tells me that it requires root to run".
Not in favor of this. It just seems to make the installation process needlessly complicated. I mean, installing an OS is an inherently privileged operation, so what security is gained by not running as root? |
@edolstra Imo this is not at all about security. Currently our live media comes without a working file manager. edit: Also, running a wayland session as root is not possible. |
@worldofpeace That's cargo cult security. If a user account can sudo to root without a password, then that user account is pretty much equivalent to root in security terms. |
<listitem> | ||
<para> | ||
The installer is no longer running the graphical session as root. | ||
To gain root privileges in the graphical session use <literal>sudo</literal> without a password. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be mentioned in the installation section of the manual as well.
Also, it probably should be sudo -i
.
Other than the file manager not being usable as root, which real other reasons are there for the installer environment to run in that double-faced manner? I will continue with arguments against, but I am open to arguments for. 1. Dolphin is fixedThis was taken from a freshly built graphical iso at the current tip of channels/nixos-18.09. We don't need this for 18.09. 2. Makes documentation harder to writeThis is because anything related to manipulations using the installer image now has to account for the possibility of having the user not be root. The existing instructions and the existing user knowledge all assume this is always done under the root user account. This will also cause issues when troubleshooting (discourse, IRC) where it will become plausible an issue a user has is caused by being in the graphical interface instead of being in a VT logged-in as root. 3. WaylandI lied, this is not an argument against, but more of an open-ended question: when is it realistic to see Wayland on the installer image? I'm not up to speed with the state of Wayland, it may be more likely than I think! I'm thinking this should be delayed, because we may need to figure out a better alternative than having the installer image sometimes be root, sometimes not be. It may be better (imho) to figure out a way so that all installer images behave the same way. Since Wayland isn't (probably) for the next release, and definitely not this one, let's work on a proactive better integrated solution instead of a reactive (to dolphin) solution. |
@samueldr I completely share your concerns. I also noticed the change in behaviour of dolphing and was deliberately holding this off for after 18.09. Now that my immediate reason for this change is no longer valid I think it's a good idea to close this and rethink how we can get a wayland session installer up and running for a future release. |
Motivation for this change
This is currently causing issues with some tools like Dolphin (the KDE
file manager) that refuses to run as root.
It's also very rare for graphical install media to be running as root nowadays, all other distros I know of run their installation processes as unprivileged users.
We can instead allow passwordless sudo with similar effect.
On the TTY you are still root, it's just the graphical session user that has changed.
I also took the opportunity to deduplicate some configuration that was the same across KDE/GNOME live ISOs.
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)